Honor qualified Cache-Control: private="field" in a shared cache (RFC9111 section 5.2.2.7) - #868
Open
arturobernalg wants to merge 1 commit into
Open
Conversation
arturobernalg
force-pushed
the
cache-qualified-private
branch
from
August 12, 2026 15:12
7e9ca32 to
e07363a
Compare
ok2c
reviewed
Aug 16, 2026
arturobernalg
force-pushed
the
cache-qualified-private
branch
from
August 16, 2026 17:32
e07363a to
a501878
Compare
ok2c
requested changes
Aug 17, 2026
| } | ||
|
|
||
| static HttpCacheEntry stripPrivateFields(final HttpCacheEntry entry) { | ||
| final ResponseCacheControl cacheControl = CacheControlHeaderParser.INSTANCE.parse(entry); |
Member
There was a problem hiding this comment.
@arturobernalg This looks conceptually wrong. Why do we need to re-parse the headers and re-create ResponseCacheControl here? In fact this is the wrong architectural layer for such a decision. This is the responsibility of the protocol layer, not of a caching backend. Pull this logic up into CachingExec / AsyncCachingExec
ok2c
requested changes
Aug 17, 2026
| } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PRIVATE)) { | ||
| builder.setCachePrivate(true); | ||
| // A repeated private directive replaces any earlier one; only the last takes effect. | ||
| privateFields.clear(); |
Member
There was a problem hiding this comment.
@arturobernalg This whole thing can be condensed into just a few lines of code. Why the parsing logic should be any different that that of CACHE_CONTROL_NO_CACHE?
| class TestAsyncCachingExecPrivateFields { | ||
|
|
||
| @Test | ||
| void testQualifiedPrivateFieldStrippedButRemainderCachedBySharedCache() throws Exception { |
Member
There was a problem hiding this comment.
@arturobernalg Does this need to be an integration test? Cannot be the same code be tested in TestCachingExecChain or TestAsyncCachingExecChain?
… 9111 section 5.2.2.7) A shared cache now honours the qualified form of the private response directive. Instead of rejecting the whole response, it stores the response with the header fields named by the directive removed from the stored copy, while the response returned to the requesting client retains them; the directive limits only where a field may be stored, not whether it may be delivered to the client that issued the request. The named fields are removed when a response is first stored, when a Vary variant is stored during negotiation, and when a stored entry is updated by a 304 revalidation. On a revalidation the fields are removed from both the 304 response and the entry it updates, so the header merge cannot carry a previously stored field into the updated entry. A bare private directive continues to make the whole response non-storable by a shared cache, and a repeated private directive is last-value-wins. The decision is taken in the protocol layer by CachingExec and AsyncCachingExec, reusing the Cache-Control already parsed for the exchange; the caching backends remain pure storage. The private field list is parsed the same way as the no-cache field list.
arturobernalg
force-pushed
the
cache-qualified-private
branch
from
August 17, 2026 18:58
a501878 to
da264e2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A shared cache now stores a response that carries a qualified private directive with the named header fields removed from the stored copy, instead of treating the whole response as
non-cacheable, while the response returned to the caller retains those fields. The fields are removed from freshly stored entries, from entries updated by a 304 revalidation, and from the root entry of a Vary response. A bare private directive still makes the whole responsenon-storableby a shared cache, and multiple qualified private directives accumulate their field names.